-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Link permissions #20846
Link permissions #20846
Conversation
/backport to stable19 |
@@ -511,6 +502,11 @@ public function createShare( | |||
$permissions = Constants::PERMISSION_READ; | |||
} | |||
|
|||
// TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones | |||
if (($permissions & Constants::PERMISSION_READ)$this->shareManager->outgoingServer2ServerSharesAllowed()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
It still doesn't change the issue about the front end then. |
Ah, actually it might 🤔 |
Not really. But the feeration part that we use instally to check this uses the ShareManager. Basically this fix is not pretty. But it makes the state consistent. The hack for the frontend stays in place for now. It is not ideal I know. |
Failing tests 🙈 |
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
73e558a
to
8289565
Compare
Rebased and pushed a fix for the failing test as well. |
Follow up to #20726
Turns out we do need the permission in the DB. Else the federated code still chokes as it doesn't use the API.
Also updated the tests to reflect the new situation.